Skip to content

Improves model info detection for custom Bedrock ARNs #3799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Jun 2, 2025
Merged

Improves model info detection for custom Bedrock ARNs #3799

merged 12 commits into from
Jun 2, 2025

Conversation

@adamhill
Copy link
Contributor

@adamhill adamhill commented May 21, 2025 •
edited by ellipsis-dev bot
Loading

Related GitHub Issue

Closes: #3712

Description

  • Adds heuristics to better estimate model capabilities when using unknown or custom model ARNs, including context window and max tokens.
  • Allows user overrides for key model parameters via provider settings, improving flexibility and reliability for non-standard model integrations.
  • Has a default size fallback for unknown ARNs

Test Procedure

Untested.- No Bedrock Credentials.

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • ✨ New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

  • Simple line that says the default context is 128K for unknown ARNs

Additional Notes

First PR unable to test w/o Bedrock credentials

Get in Touch

@GodelEscherSpock (discord)
@adamhill (github)

Important

Enhances model info detection for custom Bedrock ARNs with heuristics and user overrides, and updates ProviderSettings to include awsModelContextWindow.

  • Behavior:
    • Adds guessModelInfoFromId() in AwsBedrockHandler in bedrock.ts to estimate model capabilities for unknown/custom ARNs.
    • Allows user overrides for maxTokens and contextWindow via ProviderSettings.
    • Sets default context window to 128,000 for unknown ARNs.
  • Types:
    • Adds awsModelContextWindow to ProviderSettings in roo-code.d.ts and types.ts.
  • Misc:
    • Updates bedrockSchema in schemas/index.ts to include awsModelContextWindow.

This description was created by Ellipsis for e4a4685. You can customize this summary. It will automatically update as commits are pushed.

@adamhill
Improves model info detection for custom Bedrock ARNs
e4a4685 
Adds heuristics to better estimate model capabilities when using unknown or custom model ARNs, including context window and max tokens. Allows user overrides for key model parameters via provider settings, improving flexibility and reliability for non-standard model integrations.

Fixes RooCodeInc#3712
@adamhill adamhill requested review from cte and mrubens as code owners May 21, 2025 20:45
adamhill added 5 commits May 21, 2025 16:01
@adamhill
Merge branch 'main' into adamhill/fix-custom-ARN-in-bedrock
56f042b
@adamhill
Improves JSON syntax error handling in import flow
ec69ea2 
Provides more informative error messages for JSON syntax
errors by extracting the error position and formatting it
for clarity during import. Enhances user feedback when
invalid JSON is encountered.
@adamhill
Fixed failing tests
af0f409
@adamhill
Delete pnpm-lock.yaml
82871f8
@adamhill
Merge branch 'main' into adamhill/fix-custom-ARN-in-bedrock
94c6d02
@hannesrudolph
Copy link
Collaborator

hannesrudolph commented May 22, 2025

@mrubens LGTM

@adamhill
Added Rory's cache fix from PR RooCodeInc#3099
cc6bbb1 
PR RooCodeInc#3009 has an important fix, alerted to me by @jbbrown

It was a one liner so I pulled it in.

This brings up a question can we merge PR's in the GH UI?
@hannesrudolph hannesrudolph moved this from New to PR [Greenlit] in Roo Code Roadmap May 22, 2025
@hannesrudolph hannesrudolph moved this from TEMP to Needs Preliminary Review in Roo Code Roadmap May 27, 2025
@hannesrudolph hannesrudolph added the Followup Needs followup from support or code team label May 27, 2025
@daniel-lxs
Copy link
Member

daniel-lxs commented May 27, 2025 •
edited
Loading

Hey @adamhill, this looks good to me, I think you just need to solve the merge conflicts and we are good to go.

@daniel-lxs daniel-lxs moved this from PR [Needs Preliminary Review] to PR [Needs Review] in Roo Code Roadmap May 27, 2025
@daniel-lxs daniel-lxs removed the Followup Needs followup from support or code team label May 27, 2025
mrubens
mrubens reviewed May 28, 2025
View reviewed changes
@hannesrudolph hannesrudolph moved this from PR [Needs Review] to PR [Changes Requested] in Roo Code Roadmap May 28, 2025
@adamhill
Add Claude 4 and Opus 4 to modelID's
ec56726 
Kept previous parameters, did not see any changes in those.
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 29, 2025
@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Review] in Roo Code Roadmap May 29, 2025
mrubens
mrubens reviewed Jun 2, 2025
View reviewed changes
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has anyone tested this yet? I think we need to find someone to confirm that it works before we merge. Thank you! cc: @hannesrudolph

@daniel-lxs daniel-lxs mentioned this pull request Jun 2, 2025
Closed
@hannesrudolph
Copy link
Collaborator

hannesrudolph commented Jun 2, 2025

Has anyone tested this yet? I think we need to find someone to confirm that it works before we merge. Thank you! cc: @hannesrudolph

Rony did.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 2, 2025
@mrubens mrubens merged commit c4dab9e into RooCodeInc:main Jun 2, 2025
9 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 2, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daniel-lxs daniel-lxs daniel-lxs left review comments

@mrubens mrubens mrubens approved these changes

@cte cte Awaiting requested review from cte cte is a code owner

Assignees

No one assigned

Labels

lgtm This PR has been approved by a maintainer PR - Needs Review size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Roo Code Roadmap
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Amazon bedrock - wrong limits

4 participants

@adamhill @hannesrudolph @daniel-lxs @mrubens